fix: open browser and copy code for github copilot device oauth flow#7577
fix: open browser and copy code for github copilot device oauth flow#7577sheikhlimon wants to merge 28 commits intoblock:mainfrom
Conversation
Signed-off-by: sheikhlimon <sheikhlimon404@gmail.com>
…rowser-clipboard Signed-off-by: sheikhlimon <sheikhlimon404@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d4209eb8fd
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if clipboard.set_text(code).is_ok() { | ||
| tracing::info!("GitHub Copilot code copied to clipboard: {}", code); |
There was a problem hiding this comment.
Handle clipboard write failures in Copilot device login
This logic only falls back when Clipboard::new() fails, but if clipboard initialization succeeds and set_text fails (e.g., Wayland/X11 clipboard permission/runtime issues), no fallback path exposes the device code except println!, which is not visible in the desktop flow this change is trying to fix. In that case users can still get stuck at sign-in because they have a browser window but no code to enter.
Useful? React with 👍 / 👎.
jamadeo
left a comment
There was a problem hiding this comment.
This is a clever way around the issue, but I'm not sure I'd want goose writing directly to my clipboard.
What would it take to properly show the code in the desktop UI?
| let code = device_code_info.user_code.as_str(); | ||
|
|
||
| if let Ok(mut clipboard) = arboard::Clipboard::new() { | ||
| if clipboard.set_text(code).is_ok() { |
There was a problem hiding this comment.
We should avoid writing to the clipboard like this without an explicit action -- the user might have something there they don't want overwritten. Also, I'm not sure the user will know that his code is in their clipboard when the browser opens.
It's more work, but I would suggest looking into modifying the existing /config/providers/{provider}/oauth flow to allow rendering a code in the UI element.
There was a problem hiding this comment.
Thanks for pointing that out - the clipboard concern makes sense
From what I understand, this might involve two changes:
That way the clipboard write could be removed, since the user would be able to copy it themselves. Let me know if that sounds roughly like the direction you had in mind. |
|
@sheikhlimon yes that sounds exactly right! For the oauth handler, however we change it will still need to be compatible with the other providers using it, but this is definitely the way to go. |
Signed-off-by: sheikhlimon <sheikhlimon404@gmail.com>
Signed-off-by: sheikhlimon <sheikhlimon404@gmail.com>
Signed-off-by: sheikhlimon <sheikhlimon404@gmail.com>
Signed-off-by: sheikhlimon <sheikhlimon404@gmail.com>
Signed-off-by: sheikhlimon <sheikhlimon404@gmail.com>
Signed-off-by: sheikhlimon <sheikhlimon404@gmail.com>
Signed-off-by: sheikhlimon <sheikhlimon404@gmail.com>
Signed-off-by: sheikhlimon <sheikhlimon404@gmail.com>
Signed-off-by: sheikhlimon <sheikhlimon404@gmail.com>
Signed-off-by: sheikhlimon <sheikhlimon404@gmail.com>
Signed-off-by: sheikhlimon <sheikhlimon404@gmail.com>
Signed-off-by: sheikhlimon <sheikhlimon404@gmail.com>
Signed-off-by: sheikhlimon <sheikhlimon404@gmail.com>
Signed-off-by: sheikhlimon <sheikhlimon404@gmail.com>
Signed-off-by: sheikhlimon <sheikhlimon404@gmail.com>
Signed-off-by: sheikhlimon <sheikhlimon404@gmail.com>
Signed-off-by: sheikhlimon <sheikhlimon404@gmail.com>
…rowser open Signed-off-by: sheikhlimon <sheikhlimon404@gmail.com>
Signed-off-by: sheikhlimon <sheikhlimon404@gmail.com>
Signed-off-by: sheikhlimon <sheikhlimon404@gmail.com>
Signed-off-by: sheikhlimon <sheikhlimon404@gmail.com>
Signed-off-by: sheikhlimon <sheikhlimon404@gmail.com>
Signed-off-by: sheikhlimon <sheikhlimon404@gmail.com>
|
@jamadeo I tried implementing the approach we discussed and got close, but I’m running into some limitations and can’t get the final flow to work reliably. I don’t think I’ll be able to complete this PR properly, so it’s probably best to close it rather than leave it half-working. Happy for someone else to pick it up if needed. Also, #8019 covers what I initially aimed for and does a better job in terms of UX. |
Summary
Relates to #6957
This PR updates GitHub Copilot's OAuth device code flow to show the code in the UI instead of silently copying it to the clipboard, addressing the UX concern raised in the PR review.
Changes:
Type of Change
AI Assistance
Testing